Files that changed from the base of the PR and between 8eaa3c7 and b3be0a9.

• src/extensions/rppqt/rppqt/schedulers/main_thread.hpp (1 hunks)
• src/rpp/rpp/operators/debounce.hpp (2 hunks)
• src/rpp/rpp/operators/delay.hpp (2 hunks)
• src/rpp/rpp/operators/subscribe_on.hpp (1 hunks)
• src/rpp/rpp/operators/timeout.hpp (1 hunks)
• src/rpp/rpp/schedulers/current_thread.hpp (1 hunks)
• src/rpp/rpp/schedulers/details/worker.hpp (1 hunks)
• src/rpp/rpp/schedulers/fwd.hpp (2 hunks)
• src/rpp/rpp/schedulers/immediate.hpp (1 hunks)
• src/rpp/rpp/schedulers/new_thread.hpp (4 hunks)
• src/rpp/rpp/schedulers/run_loop.hpp (1 hunks)
• src/rpp/rpp/schedulers/test_scheduler.hpp (5 hunks)
• src/rpp/rpp/schedulers/thread_pool.hpp (1 hunks)
• src/rpp/rpp/sources/from.hpp (2 hunks)
• src/rpp/rpp/sources/interval.hpp (2 hunks)
• src/tests/rpp/test_scheduler.cpp (4 hunks)

src/rpp/rpp/schedulers/details/worker.hpp (2)

Line range hint 1-1: Verify the impact of removing the get_disposable function.

The get_disposable function, which returned a disposable_wrapper based on the type of Strategy, has been entirely removed. This change suggests a simplification of the class's interface, potentially indicating that the disposable functionality is no longer necessary or has been refactored elsewhere.

Run the following script to verify the impact of the removal:

---

Line range hint 1-1: Verify the impact of removing the is_none_disposable constant.

The is_none_disposable constant, which determined if the Strategy type was non-disposable, has been entirely removed. This change further reinforces the simplification of the class by reducing its public interface and possibly altering how disposable resources are managed within the scheduling strategy.

Run the following script to verify the impact of the removal:

Verification successful

---

Removal of is_none_disposable Constant Verified

The is_none_disposable constant was removed as part of a broader change related to disposables, as indicated by the commit message "remove disposables." There are no references or dependencies on this constant elsewhere in the codebase, confirming that its removal does not impact other parts of the codebase. The change simplifies the class by reducing its public interface.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of removing the `is_none_disposable` constant.

# Test: Search for the usage of the removed constant. Expect: No occurrences.
rg --type cpp $'is_none_disposable'

Length of output: 34

---

Script:

#!/bin/bash
# Description: Search for comments or documentation mentioning `is_none_disposable` and check for related changes.

# Search for comments or documentation mentioning `is_none_disposable`.
rg --type cpp --type md --type txt 'is_none_disposable'

# Check the git history for any commits related to the removal of `is_none_disposable`.
git log -S 'is_none_disposable' --oneline

Length of output: 162

src/rpp/rpp/operators/subscribe_on.hpp (1)

45-45: Verify the impact of the change on the subscription mechanism and disposable management.

The change simplifies the updated_disposable_strategy type alias by directly assigning Prev to it, removing the conditional logic that previously determined the value based on the worker's state.

This change may alter the expected behavior of the subscription mechanism, particularly in relation to disposable management.

Run the following script to verify the impact of the change:

Verification successful

---

Verification successful: No issues found with the change in updated_disposable_strategy.

The change simplifies the updated_disposable_strategy type alias by directly assigning Prev, which is consistent with its usage in other parts of the codebase. There is no evidence of the old conditional logic affecting the subscription mechanism or disposable management.

• The change aligns with similar patterns in other files.
• No occurrences of the old conditional logic were found in the subscribe method.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the impact of the change on the subscription mechanism and disposable management.

# Test 1: Search for the usage of `updated_disposable_strategy`. Expect: No occurrences of the old conditional logic.
rg --type cpp -A 5 $'updated_disposable_strategy'

# Test 2: Search for the usage of `subscribe` method. Expect: No occurrences of the old conditional logic.
rg --type cpp -A 10 $'void subscribe\('

Length of output: 41537

src/rpp/rpp/schedulers/immediate.hpp (1)

Line range hint 1-1: LGTM: The removal of the get_disposable() function is approved.

The removal of the get_disposable() function is consistent with the PR objective of removing disposables from the scheduler worker. It indicates a simplification of the interface and a change in how disposables are managed within the scheduling context.

src/rpp/rpp/schedulers/thread_pool.hpp (1)

54-54: LGTM!

The code changes are approved.

src/rpp/rpp/schedulers/test_scheduler.hpp (11)

25-25: LGTM!

The removal of inheritance from rpp::details::base_disposable simplifies the state struct and aligns with the PR objective.

---

52-52: LGTM!

Using std::max to ensure the scheduled time point is not earlier than the current time is a good practice to maintain the integrity of the scheduling logic.

---

53-53: LGTM!

Using schedulings.back() as the time point for scheduling the task maintains consistency between the schedulings vector and the actual scheduled tasks.

---

66-66: LGTM!

Using a std::weak_ptr<state> in the worker_strategy constructor aligns with the removal of disposable functionality and helps prevent potential memory leaks or dangling pointers.

---

76-76: LGTM!

Removing the disposal check before scheduling tasks aligns with the changes made in the state struct and simplifies the scheduling logic, consistent with the PR objective.

---

77-77: LGTM!

Calling the drain method after scheduling a task ensures that any tasks scheduled at or before the current time are executed immediately, maintaining the expected behavior of the scheduler.

---

84-84: LGTM!

Using a std::weak_ptr<state> for the m_state member variable in the worker_strategy class aligns with the removal of disposable functionality and helps prevent potential memory leaks or dangling pointers, consistent with the constructor modification.

---

91-91: LGTM!

Using m_state directly in the create_worker method reflects the new ownership semantics and simplifies the code, consistent with the overall shift towards using shared and weak pointers for managing the lifecycle of the state object.

---

94-94: LGTM!

Directly dereferencing m_state in the get_schedulings method simplifies the code and enhances readability, consistent with the shift towards using shared pointers for managing the state object.

---

95-95: LGTM!

Directly dereferencing m_state in the get_executions method simplifies the code and enhances readability, consistent with the shift towards using shared pointers for managing the state object.

---

106-106: LGTM!

Using a std::shared_ptr<state> for the m_state member variable in the test_scheduler class aligns with the removal of disposable functionality and simplifies the code, consistent with the overall shift towards using shared and weak pointers for managing the lifecycle of the state object.

src/extensions/rppqt/rppqt/schedulers/main_thread.hpp (1)

Line range hint 1-1: LGTM: The removal of get_disposable() simplifies the public interface.

The removal of the get_disposable() function from the public interface of main_thread_scheduler seems to be a valid simplification of the interface. The change does not appear to affect the core scheduling functionality, which is handled by the worker_strategy class.

src/rpp/rpp/sources/interval.hpp (2)

Line range hint 38-42: LGTM!

The code changes are approved. Removing the conditional block simplifies the subscription process and may potentially improve performance by eliminating unnecessary checks.

---

32-32: Verify the impact of simplifying the expected_disposable_strategy type alias.

The change removes the conditional logic that checked the worker's disposability and directly assigns the type alias to rpp::details::observables::bool_disposable_strategy_selector. This simplifies the code but may impact how observers are managed in the context of scheduling.

Run the following script to verify the usage of expected_disposable_strategy and bool_disposable_strategy_selector in the codebase:

Verification successful

---

Verified: Simplification of expected_disposable_strategy aligns with design choices.

The change to use rpp::details::observables::bool_disposable_strategy_selector in interval_strategy is consistent with similar usages across the codebase. This suggests a deliberate design choice to standardize disposable strategies in these contexts, minimizing the impact on the overall behavior. The change is verified and aligns with the broader design strategy.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `expected_disposable_strategy` and `bool_disposable_strategy_selector`.

# Test 1: Search for the usage of `expected_disposable_strategy`. 
# Expect: Only occurrences in the context of `interval_strategy`.
rg --type cpp -A 5 $'expected_disposable_strategy'

# Test 2: Search for the usage of `bool_disposable_strategy_selector`. 
# Expect: Occurrences in the context of `interval_strategy` and other relevant places.
rg --type cpp -A 5 $'bool_disposable_strategy_selector'

Length of output: 25542

src/rpp/rpp/schedulers/run_loop.hpp (1)

Line range hint 1-1: LGTM: The removal of the get_disposable() method simplifies the interface.

The get_disposable() method has been removed from the worker_strategy class. This change simplifies the interface by eliminating a potentially redundant or unnecessary method, as the concept of a disposable resource may no longer be relevant or needed in this context. The remaining code does not seem to be affected by this removal.

src/rpp/rpp/schedulers/new_thread.hpp (12)

32-32: LGTM!

The change from disposable to state_t is approved as it aligns with the new approach to thread lifecycle management.

---

35-35: LGTM!

The default constructor for state_t is approved.

---

Line range hint 37-52: LGTM!

The destructor of state_t is approved as it correctly handles the thread lifecycle based on the new is_stoping flag.

---

44-44: LGTM!

The change from is_disposed to is_stoping is approved as it aligns with the new approach to thread lifecycle management.

---

54-60: LGTM!

Moving the defer_to method into the state_t class is approved as it improves the encapsulation of task queuing within the state.

---

62-67: LGTM!

Introducing the queue_data struct is approved as it enhances clarity by separating the concerns of task management and state handling.

---

69-69: LGTM!

Updating the data_thread method to use the new queue_data structure is approved as it ensures that the thread operates on the correct state representation.

---

76-77: LGTM!

Updating the condition check to use the new is_stoping flag is approved as it ensures that the thread exits correctly based on the updated state management logic.

---

79-79: LGTM!

Updating the condition check to use the new is_stoping flag is approved as it ensures that the thread waits correctly based on the updated state management logic.

---

123-123: LGTM!

Updating the member variable to use the new queue_data structure is approved as it ensures that the state is correctly managed within the class.

---

139-140: LGTM!

Updating the defer_to method to use the new state_t class is approved as it ensures that the tasks are correctly queued within the worker strategy.

---

145-145: LGTM!

Updating the member variable to use the new state_t class is approved as it ensures that the state is correctly managed within the worker strategy.

src/rpp/rpp/schedulers/fwd.hpp (1)

147-149: LGTM! The changes simplify the strategy concept and align with the PR objective.

The changes to the strategy concept in the rpp::schedulers::constraint namespace are approved for the following reasons:

1. The removal of the requirement related to the get_disposable() method is consistent with the PR objective of removing disposables from the scheduler worker. This simplifies the constraints on the strategy concept.

2. The modification of the requires clause to remove the parameters indicates a shift in the expected conditions for the concept. This change reduces the strictness of the strategy concept, potentially allowing a broader range of types to satisfy it.

The overall effect of these changes is a simplification of the strategy concept, which aligns with the goal of the PR.

src/rpp/rpp/operators/debounce.hpp (1)

170-170: Simplified container type alias.

The change simplifies the container type alias by directly using DisposableStrategy::disposable_container, removing the conditional logic based on the worker's disposability. This aligns with the overall refactoring goal of simplifying the logic related to disposable workers.

How does this change impact the instantiation and utilization of the disposable container within the debounce operator? Please provide more context on the implications of this simplification.

src/rpp/rpp/sources/from.hpp (1)

87-87: LGTM!

The change to expected_disposable_strategy type alias aligns with the PR objective of removing disposables from the scheduler worker. It simplifies the logic by always using rpp::details::observables::bool_disposable_strategy_selector.

src/rpp/rpp/operators/timeout.hpp (2)

136-138: LGTM!

The changes are approved:

• The addition of the using container declaration is a minor change that does not affect the functionality.
• The modification to the timeout calculation is a simplification that reduces dependency on a utility function, which is a positive change.

---

145-145: LGTM!

The change is approved:

• The addition of the using wrapper declaration is a minor change that does not affect the functionality.

src/tests/rpp/test_scheduler.cpp (2)

356-356: LGTM!

The code change is approved. The removal of the get_disposable() method call is consistent with the simplification mentioned in the summary.

---

356-356: LGTM!

The code change is approved. Storing the worker in an std::optional allows better control over its lifecycle within the test.

Files that changed from the base of the PR and between b3be0a9 and f5d12ed.

• src/rpp/rpp/schedulers/test_scheduler.hpp (5 hunks)

• src/rpp/rpp/schedulers/test_scheduler.hpp